Add the WASI runtime test target #977

mkhamoyan · 2022-11-29T09:51:56Z

Add WASI runtime as XHarness target

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/DirectoryArgument.cs

README.md

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/ErrorPatternsFileArgument.cs

radical

Good first draft 👍

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/ErrorPatternsFileArgument.cs

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/SymbolMapFileArgument.cs

radical · 2022-12-01T17:48:02Z

...osoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/SymbolicatePatternsFileArgument.cs

+using System;
+using System.IO;
+
+namespace Microsoft.DotNet.XHarness.CLI.CommandArguments.Wasi;


This doesn't need to be duplicated, and can be shared by Wasi, and Wasm commands.

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/SymbolicatorArgument.cs

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/ErrorPatternScanner.cs

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/Engine/WasiTestCommand.cs

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/WasmEngineArgument.cs

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/LibraryArgument.cs

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/DirectoryArgument.cs

tests/Microsoft.DotNet.XHarness.CLI.Tests/Commands/XHarnessCommandTests.cs

radical

This is looking much better now! Some more feedback based on trying out the new command.

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/WasmEngineArgument.cs

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/WasiTestCommandArguments.cs

radical · 2022-12-07T21:02:52Z

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/WasiTestCommandArguments.cs

+    public WasmEngineArgument Engine { get; } = new();
+    public WasmEngineLocationArgument EnginePath { get; } = new();
+    public WasmEngineArguments EngineArgs { get; } = new();
+    public WasmFileArgument WasmFile { get; } = new("dotnet.wasm");  


This is not needed. It can be specified as part of the pass through commands. For example:

xharness wasi test -- WasmTestRunner.wasm System.Collections.Tests.dll

All the arguments after -- are pass-through arguments, which will be passed to wasmtime as-is.

maybe we could try to add https://wasmedge.org/ engine to see if they have same expectations

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/WasiTestCommandArguments.cs

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/Engine/WasiTestCommand.cs

radical · 2022-12-07T21:12:18Z

It would be good to use FindEngineInPath in the test command (#977 (comment)).

radical · 2022-12-09T15:22:02Z

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/Engine/WasiTestCommand.cs

+        logger.LogInformation($"Using wasm engine {Arguments.Engine.Value} from path {engineBinary}");
+        await PrintVersionAsync(Arguments.Engine.Value.Value, engineBinary);


Move these inside the try/catch, so a missing engine error can be reported correctly. Do the same for WasmTestCommand.

radical · 2022-12-09T15:23:32Z

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/Engine/WasiTestCommand.cs

+                if (logProcessor.LineThatMatchedErrorPattern != null)
+                {
+                    logger.LogError("Application exited with the expected exit code: {result.ExitCode}."
+                                    + $" But found a line matching an error pattern: {logProcessor.LineThatMatchedErrorPattern}");
+                    return ExitCode.APP_CRASH;
+                }


This is not needed because we are not using a error patterns scanner at all.

radical · 2022-12-09T15:24:37Z

src/Microsoft.DotNet.XHarness.CLI/Commands/WASI/Engine/WasiTestCommand.cs

+            var stdoutFilePath = Path.Combine(Arguments.OutputDirectory, "wasi-console.log");
+            File.Delete(stdoutFilePath);
+
+            var logProcessor = new WasmTestMessagesProcessor(xmlResultsFilePath,


The default value for the last two parameters can be made null now.

radical

Other than the few comments, looks good 👍

.

radical

LGTM 👍 great job:)

pavelsavara

thanks!

mkhamoyan added 8 commits November 21, 2022 11:32

Added wasi files

bf34885

Added wasi files

9de2255

Removed not needed files

9ba4a17

fix

07d938f

Changed hardcoded path

960dd73

Removed not needed files and added dir

fd5d872

Removed some files

eafbf96

Removed log text

a708cb0

mkhamoyan requested review from pavelsavara, radical, steveisok and thaystg December 1, 2022 10:48

pavelsavara reviewed Dec 1, 2022

View reviewed changes

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/DirectoryArgument.cs Outdated Show resolved Hide resolved

pavelsavara reviewed Dec 1, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

pavelsavara reviewed Dec 1, 2022

View reviewed changes

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/WASI/Arguments/ErrorPatternsFileArgument.cs Outdated Show resolved Hide resolved

radical requested changes Dec 1, 2022

View reviewed changes

mkhamoyan and others added 12 commits December 6, 2022 14:50

Refactored

aeff3df

Remove WebServer for wasi

613849f

Delete XHarnessCommandTests.cs

253afd0

Added back XharnessCommandTests

0cc4a66

Renamed back some of the wasm files

7d7fbf8

Fix line endings

d2170be

Delete WebServer file

ad41692

WebServer files changed

bc1f90d

Removed some files

bf185c6

Add engineargs for wasi

590c355

Removed symbolicator from wasi

e2a275f

Removed not needed code

5661f19

mkhamoyan requested review from pavelsavara and radical December 7, 2022 17:36

radical previously requested changes Dec 7, 2022

View reviewed changes

Refactored and removed not needed parts

50c616e

pavelsavara mentioned this pull request Dec 8, 2022

[WASI] experiment - tracking dotnet/runtime#65895

Open

34 tasks

Removed WasmFileArgument

fd29c39

mkhamoyan requested a review from radical December 8, 2022 15:38

radical reviewed Dec 9, 2022

View reviewed changes

Refactored

2b41f7e

mkhamoyan requested a review from radical December 13, 2022 08:35

radical approved these changes Dec 13, 2022

View reviewed changes

pavelsavara approved these changes Dec 14, 2022

View reviewed changes

premun changed the title ~~Xharness wasi~~ Add the WASI runtime test target Dec 14, 2022

mkhamoyan marked this pull request as ready for review December 15, 2022 08:06

mkhamoyan enabled auto-merge (squash) December 15, 2022 08:14

Merge branch 'dotnet:main' into xharness_wasi

56a294e

mkhamoyan disabled auto-merge December 15, 2022 08:56

mkhamoyan enabled auto-merge (squash) December 15, 2022 08:59

mkhamoyan merged commit a54eccd into dotnet:main Dec 15, 2022

		logger.LogInformation($"Using wasm engine {Arguments.Engine.Value} from path {engineBinary}");
		await PrintVersionAsync(Arguments.Engine.Value.Value, engineBinary);

Add the WASI runtime test target #977

Add the WASI runtime test target #977

Uh oh!

Conversation

mkhamoyan commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

radical Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

radical Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

pavelsavara Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical commented Dec 7, 2022

Uh oh!

radical Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

radical Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

radical Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

pavelsavara left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkhamoyan commented Nov 29, 2022 •

edited

Loading